Conversation
There was a problem hiding this comment.
Pull request overview
Introduces first-class storage for school email domains by adding a dedicated table/model and wiring it into the School domain model, with accompanying specs.
Changes:
- Added
school_email_domainstable with a composite unique index per school. - Added
SchoolEmailDomainmodel withbefore_validationnormalization (strip leading@, downcase). - Updated
Schooltohas_many :school_email_domainsand added/updated model specs.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/models/school_spec.rb | Adds association + dependent-destroy coverage for school_email_domains. |
| spec/models/school_email_domain_spec.rb | Adds model spec coverage for domain normalization behavior. |
| db/schema.rb | Reflects new school_email_domains table and foreign key. |
| db/migrate/20260420104937_create_school_email_domains.rb | Creates the new school_email_domains table and composite unique index. |
| app/models/school_email_domain.rb | Introduces the new model and domain normalization callback. |
| app/models/school.rb | Adds has_many :school_email_domains, dependent: :destroy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a migration to create the school_email_domains table, storing a domain string against a school_id
Create the model and add initial specs
Removing leading @ symbols and lower case domains
PetarSimonovic
force-pushed
the
store-school-email-domains
branch
from
4740867 to
aff003a
Compare
Test coverage89.5% line coverage reported by SimpleCov. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fspeirs
left a comment
fspeirs
left a comment
There was a problem hiding this comment.
Overall this looks like the right approach. I have one consideration about validating domains - I feel like it would be helpful to be stricter in checking that the user hasn't typo'ed their domain or misunderstood what we are asking for.
Status
Closes #1325
What's changed?
school_email_domainstableSchoolEmailDomainmodelSchool